Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[refactor] #4174: Stop encoding X25519 keys as Ed25519 #4275

Merged
merged 1 commit into from
Feb 14, 2024
Merged

[refactor] #4174: Stop encoding X25519 keys as Ed25519 #4275

merged 1 commit into from
Feb 14, 2024

Conversation

Arjentix
Copy link
Contributor

Description

  • Key exchanging trait updated with keys associated types
  • X25519 keys are now used as a separate type
  • KeyGenOption now accepts optional generic

Linked issue

Benefits

More robust type-system, less error-handling, simpler ed25519 code

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@Arjentix Arjentix added iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality crypto labels Feb 12, 2024
@Arjentix Arjentix self-assigned this Feb 12, 2024
@VAmuzing VAmuzing self-assigned this Feb 13, 2024
@Erigara Erigara self-assigned this Feb 13, 2024
@coveralls
Copy link

coveralls commented Feb 13, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 7898328175

Details

  • -36 of 62 (41.94%) changed or added relevant lines in 5 files are covered.
  • 1230 unchanged lines in 22 files lost coverage.
  • Overall coverage decreased (-0.009%) to 56.779%
Changes Missing Coverage Covered Lines Changed/Added Lines %
crypto/src/lib.rs 1 2 50.0%
crypto/src/kex/x25519.rs 18 30 60.0%
p2p/src/peer.rs 0 23 0.0%
Files with Coverage Reduction New Missed Lines %
core/src/query/store.rs 1 73.2%
crypto/src/kex/x25519.rs 1 66.1%
crypto/src/signature/secp256k1.rs 1 91.94%
core/src/smartcontracts/isi/mod.rs 8 76.67%
data_model/src/predicate.rs 9 86.83%
core/src/smartcontracts/isi/tx.rs 13 66.67%
smart_contract/executor/derive/src/default.rs 15 0.0%
core/src/wsv.rs 17 52.2%
core/src/queue.rs 18 93.52%
core/src/tx.rs 26 53.18%
Totals Coverage Status
Change from base Build 7884695009: -0.009%
Covered Lines: 22764
Relevant Lines: 40092
💛 - Coveralls

Erigara
Erigara previously approved these changes Feb 14, 2024
View reviewed changes
mversic
mversic previously approved these changes Feb 14, 2024
View reviewed changes
@Arjentix Arjentix dismissed stale reviews from mversic and Erigara via 631322a February 14, 2024 08:19
@Arjentix Arjentix requested review from mversic and Erigara February 14, 2024 08:20
@Arjentix Arjentix merged commit 960f657 into hyperledger-iroha:iroha2-dev Feb 14, 2024
10 checks passed
@Arjentix Arjentix deleted the x25519_strong_type branch February 14, 2024 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Erigara Erigara Erigara approved these changes

@mversic mversic mversic approved these changes

@s8sato s8sato Awaiting requested review from s8sato

@SamHSmith SamHSmith Awaiting requested review from SamHSmith

@QuentinI QuentinI Awaiting requested review from QuentinI

@outoftardis outoftardis Awaiting requested review from outoftardis

@ilchu ilchu Awaiting requested review from ilchu

@pesterev pesterev Awaiting requested review from pesterev

@0x009922 0x009922 Awaiting requested review from 0x009922

@DCNick3 DCNick3 Awaiting requested review from DCNick3

Assignees

@VAmuzing VAmuzing

@Arjentix Arjentix

@Erigara Erigara

Labels
crypto iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Arjentix @coveralls @Erigara @mversic @VAmuzing